-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extension to mpi programs #351
base: master
Are you sure you want to change the base?
Conversation
I know is a big commit. I will first start to pull from the actual master. But would be nice to have a point of discussion in case there is interest to merge in the actual gdbgui. |
Merge from base
ok I started to merge and I found the first problem when I try to use the server ... File ".../gdbgui/gdbgui/server/sessionmanager.py", line 9, in Where do I get IoManager ? I do not see in the repo of pygdbmi |
It is in add_reader branch ... of pygdbmi ... and is failing in your CI Also I am not able to make this last refactor working: The server die with: File "/home/i-bird/Desktop/MOSAIC/OpenFPM_project/gdbgui/gdbgui/cli.py", line 254, in main Not sure how it should works. OK fixed with import gdbgui.server.server in line 25 of cli.py |
gdbgui/src/js/Actions.js
Outdated
jQuery.ajax({ | ||
url: "mpi_processes_info", | ||
success: function(data, status) { | ||
data_lines = data.split(/\r?\n/); | ||
}, | ||
async: false | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to eventually replace all jQuery.ajax calls with fetch.
It would be helpful if this was switched to use fetch, and the function turned into an async
function. If you don't know what that means, don't worry about it. I will just fix it in a future update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will have a look. My programming skill in python and Javascript ( now TypeScript ) are basic, I am learning while going.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s fine to leave as is. I can migrate it along with the other calls at a later time.
gdbgui/src/js/GdbApi.jsx
Outdated
* @param state mpi state | ||
* @return nothing | ||
*/ | ||
set_mpi_state: function(state) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All files are now typescript, so arguments like state
can be annotate with types now. I will be working to update existing code, but would like to see new code annotated from the start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I saw merging with master ... I am already changing all of them doing it.
Hi, thanks for the PR! Very impressive. I just landed a whole bunch of changes to gdbgui recently. Unfortunately those changes have caused conflicts with this PR. Most of the code that was changed was on the backend. The frontend code was not affected very much other than the switch from .js/.jsx to .ts/.tsx. I just merged the pygdbmi add-reader branch into master and published a new version of pygdbmi (0.10.0.0) which is required by the latest version of gdbgui (0.14.0.0). I looked at your screenshot in the related issue. The only thing I have to suggest is to move the processor selection to the right pane. I am looking forward to seeing the merge conflicts resolved and giving it a try. I am curious -- is there a particular project or company these changes already being used in, or did you make this just in case you will need it in the future? |
gdbgui-mpi/__main__.py
Outdated
@@ -0,0 +1,15 @@ | |||
from gdbgui import backend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this new file needed or can the existing cli.py
be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably is removable, I will have a look
@@ -0,0 +1,3 @@ | |||
#! /bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the gdbgui-mpi
folder might fit better in examples/mpi
. There are already examples in the examples
folder.
gdbgui/src/js/ProcessorsButtons.jsx
Outdated
store.connectComponentState(this, ["processors_states"]); | ||
} | ||
render() { | ||
let btn_class = "btn btn-default btn-sm"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bootstrap classes are being phased out in favor of tailwind css.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don’t know tailwind that’s is okay. I will update it all later.
gdbgui/src/js/SourceCode.jsx
Outdated
@@ -165,8 +169,10 @@ class SourceCode extends React.Component { | |||
) { | |||
let row_class = ["srccode"]; | |||
|
|||
if (is_gdb_paused_on_this_line) { | |||
if (is_gdb_paused_on_this_line === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a boolean, it doesn't make sense to compare to a number. If the semantics around this have changed, the variable name and type should change.
noxfile.py
Outdated
@@ -13,7 +13,7 @@ | |||
|
|||
@nox.session(reuse_venv=True, python=python) | |||
def tests(session): | |||
session.install(".", "pytest", "pytest-cov") | |||
session.install(".", "pytest", "pytest-cov", "flask-socketio>4.2.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"flask-socketio
should already be included (if necessary) with the .
target
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will check again the problem of not having the > 4.2 it was that it was installing the version 2.9.3 (I think). And I needed flask-socketio bigger than 3.3 because of test_backend_mpi.py. I introduced a test that to emulate a debug session for the server and I had to connect the HTTP cookies of the flask client to the flask-io socket. This feature was introduced in flask-socketio 3.3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the version is upgraded (and everything still works) that is fine, but it should be done in setup.py, which is the where Gdbgui defines its dependencies.
We are a research group specialized in Parallel simulations and I am one of the software developer in HPC. From long time I was searching for a small project that could the adapted to become an opensource parallel debugger for the HPC community. And this fullfill all the requirements. In few days was possible to create something working |
Do you have a website I could check out? |
While for the simulation library |
…on/gdbgui into extension_to_mpi_programs
docs/changelog.md
Summary of changes
This diff add support to work with mpi programs in Gdbgui.
The main changes regard adding the mpi_rank element to debug session a way to search the gdbsession by rank extend run_gdb_command with the process id that is default to -1. -1 in MPI based debug session -1 mean run the command on all sessions. in case of non-MPI is ingnored and -1 mean run on the single underline gdb session. An additional HTML page on the server to give to the client access to the MPI servers informations. Process information is attached to all gdb response answer in case of non MPI process information is -1. process information has been also attached to some function in Actions now process information of the message process, but because it is a defaulted to -1 so should not produce problem if there is code around calling the function in the old form.
inferior_program_paused has been changed to process processor information
change_process_on_focus: and refresh_state_for_gdb_pause has been added to process change of processor on focus and update of the GUI information
connect_to_gdbserver_mpi is added to handle the connection to MPI gdb server
Several global variables to initial_store_data has been added to store information for each processor
A processo status bar has been added when MPI programs has been debugged
gdbgui/src/js/Breakpoints.tsx has been fixed to remove breakpoint placed in templated code
All around there are change to handle process information, when needed
Test plan
2 tests has been added one for the JSX side that test server + Gdbgui TypeScript code using puppeteer and chrome headless, emulating click events and keyboard typing. The test go trough a full debug session.
The second test test is for the python server the test send command directly to the server to go through a debug session.
Tested by running